-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Remove MultiCollectorWrapper and use MultiCollector in Lucene instead #19595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove MultiCollectorWrapper and use MultiCollector in Lucene instead #19595
Conversation
Signed-off-by: Binlong Gao <[email protected]>
Signed-off-by: Binlong Gao <[email protected]>
|
❌ Gradle check result for db44cf9: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19595 +/- ##
============================================
+ Coverage 73.06% 73.12% +0.05%
+ Complexity 70630 70604 -26
============================================
Files 5723 5722 -1
Lines 323513 323503 -10
Branches 46852 46851 -1
============================================
+ Hits 236384 236556 +172
+ Misses 68013 67879 -134
+ Partials 19116 19068 -48 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
server/src/main/java/org/opensearch/search/query/TopDocsCollectorContext.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/opensearch/search/query/TopDocsCollectorContext.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Binlong Gao <[email protected]>
|
❕ Gradle check result for 6fd02e5: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
sandeshkr419
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a minor code refactoring near the lines of code you are touching upon!
server/src/main/java/org/opensearch/search/query/TopDocsCollectorContext.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Binlong Gao <[email protected]>
server/src/main/java/org/opensearch/search/query/TopDocsCollectorContext.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Binlong Gao <[email protected]>
…opensearch-project#19595) * Remove MultiCollectorWrapper and use MultiCollector in Lucene instead Signed-off-by: Binlong Gao <[email protected]> * Modify change log Signed-off-by: Binlong Gao <[email protected]> * Optimize some code Signed-off-by: Binlong Gao <[email protected]> * pattern matching for instanceof Signed-off-by: Binlong Gao <[email protected]> * Remove redundant bracket Signed-off-by: Binlong Gao <[email protected]> --------- Signed-off-by: Binlong Gao <[email protected]>
…opensearch-project#19595) * Remove MultiCollectorWrapper and use MultiCollector in Lucene instead Signed-off-by: Binlong Gao <[email protected]> * Modify change log Signed-off-by: Binlong Gao <[email protected]> * Optimize some code Signed-off-by: Binlong Gao <[email protected]> * pattern matching for instanceof Signed-off-by: Binlong Gao <[email protected]> * Remove redundant bracket Signed-off-by: Binlong Gao <[email protected]> --------- Signed-off-by: Binlong Gao <[email protected]>
Description
As this comment said, once MultiCollector.getCollectors() method in Lucene is declared as public, we can use that method directly rather than creating a new similar class, this PR replaces MultiCollectorWrapper with MultiCollector in Lucene, and removes MultiCollectorWrapper as it was internally used and won't be used anymore.
Related Issues
No issue.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.